Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove separate signature checking thread classes #4267

Merged
merged 5 commits into from
Nov 6, 2023

Conversation

clemahieu
Copy link
Contributor

@clemahieu clemahieu commented Aug 22, 2023

This removes the use of separate signature-checking threads/classes from the node.
These classes add complexity to critical code. Even where performance optimizations could be made through multi-threading, it would be better implemented using standard C++ instead of a custom class.
It's unclear if these classes are helping performance at all so we're opting to remove them until/if a performance improvement is needed.

This PR contains 4 commits:

  • Adds explicit negative tests for state and epoch blocks to verify incorrect signatures are rejected
  • All blocks passed to block_processor::add_impl are enqueued in the block processor, rather than splitting them based on the type, and passed to separate signature checking threads. This remove use of signature checking threads from block_processor.
  • Remove use of signature checking threads from vote_processor and check each vote directly in the vote_processor thread
  • Remove unused signature checking classes.

@clemahieu clemahieu force-pushed the remove_signature_threads branch 2 times, most recently from 530d91e to 32a2e4e Compare August 22, 2023 15:14
@clemahieu clemahieu marked this pull request as ready for review August 22, 2023 16:25
@qwahzi qwahzi added this to the V26.0 milestone Sep 25, 2023
pwojcikdev
pwojcikdev previously approved these changes Oct 19, 2023
Copy link
Contributor

@pwojcikdev pwojcikdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good PR that removes unnecessary complexity, all the checks were performed inside the ledger_processor anyway.

pwojcikdev
pwojcikdev previously approved these changes Nov 5, 2023
…or::blocks regardless of type.

Negative testing for signatures of state/epoch blocks is done with ledger.fail_state_bad_signature and ledger.fail_epoch_bad_signature. Signature checking is always done within the block verification functions themselves since nanocurrency#4021
pwojcikdev
pwojcikdev previously approved these changes Nov 6, 2023
@clemahieu clemahieu merged commit ef0dde6 into nanocurrency:develop Nov 6, 2023
14 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged / V26.0
Development

Successfully merging this pull request may close these issues.

3 participants